feat: add eth2wrap/version#121
Conversation
- Avoids parsing on each request
- Prevents VSCode from rebuilding everything during tests
There was a problem hiding this comment.
Pull request overview
This PR adds beacon node version validation functionality to the charon crate by implementing a new eth2wrap/version module. The implementation validates Beacon Node client versions against defined minimum versions using the existing charon_core::version module and regex pattern matching.
Changes:
- Adds version validation for six Beacon Node clients (Lighthouse, Teku, Lodestar, Nimbus, Prysm, and Grandine)
- Implements regex-based version extraction and semantic version comparison
- Includes comprehensive test coverage for version validation logic
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| flake.nix | Sets RUSTC_BOOTSTRAP=1 environment variable for the development shell |
| crates/charon/src/lib.rs | Adds public module declaration for eth2wrap |
| crates/charon/src/eth2wrap/version.rs | Implements beacon node version checking with validation logic and tests |
| crates/charon/src/eth2wrap/mod.rs | Module file declaring the version submodule |
| crates/charon/Cargo.toml | Adds regex dependency |
| Cargo.toml | Moves regex from dev-dependencies to workspace dependencies |
| Cargo.lock | Updates dependency versions (primarily Alloy crates and regex placement) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if client < minimum { | ||
| return Err(BeaconNodeVersionError::TooOld { client, minimum }); | ||
| } | ||
|
|
There was a problem hiding this comment.
There is a bad version checking: https://github.com/ObolNetwork/charon/blob/main/app/eth2wrap/version.go#L70-L74
But when i searched the golang implementation, it seems there are no places modify this map. Do we want to add that implementation here?
There was a problem hiding this comment.
I did not include it since - as you mention - there is no way at runtime to modify this map, and currently it's empty, so in practice is essentially dead code.
|
|
||
| let name = &matches[1]; | ||
| let minimum = MINIMUM_BEACON_NODE_VERSIONS | ||
| .get(&name.to_lowercase().as_str()) |
There was a problem hiding this comment.
nit: golang implementation is case sensitive
There was a problem hiding this comment.
Good catch, but I think it's safe to use lowercase everywhere (could be an issue on their side).
Solves #153
Validates Beacon Node client versions by leveraging the
charon_core::versionmodule.There is an issue with the
Grandineversion used in the original Go implementation that is impossible to parse using theSemVer::parsemethod (https://github.com/ObolNetwork/charon/blob/941bb212f3065ef3551e636e7c3a1965324c21e0/app/eth2wrap/version.go#L20). This was reported to the Obol team already and the issue seems to be upstream: the Grandine team is not following the correct semver naming format.On our side, the fix implies changing
v2.0.0.rc0tov2.0.0-rc0